Provide a default value for server_name to LoggingContext
#19003
Conversation
This is a backwards-compatibility patch for modules that imported LoggingContext from within Synapse. See matrix-org/synapse-s3-storage-provider#133
anoadragon453
changed the title
server_nameserver_name to LoggingContext
| *, | ||
| name: str, | ||
| server_name: str, | ||
| server_name: str = "SERVER_NAME_NOT_PROVIDED", |
There was a problem hiding this comment.
While, I didn't forsee this problem when making the change, setting a default encourages people not to set this appropriately even in our own Synapse codebase. We could add a lint for our own codebase 🤔
Should downstream consumers really be using LoggingContext? Probably not
This basically the same reasoning I used in #18989 (comment)
[...] we don't expect (and shouldn't support) these things to be used externally.
I would much rather push harder down this route if we're willing (which means this PR isn't necessary).
| *, | ||
| name: str, | ||
| server_name: str, | ||
| server_name: str = "SERVER_NAME_NOT_PROVIDED", |
There was a problem hiding this comment.
| server_name: str = "SERVER_NAME_NOT_PROVIDED", | |
| server_name: str = "unknown_server_because_not_provided", |
To better match what we have in the codebase:
unknown_server_from_sentinel_contextunknown_server_from_no_contextsynapse_module_running_from_unknown_server
We could go even further if we add a lint to enforce server_name is set within Synapse as then we know this only happens from third-party code:
| server_name: str = "SERVER_NAME_NOT_PROVIDED", | |
| server_name: str = "unknown_server_from_third_party_code", |
This matches the existing synapse_module_running_from_unknown_server:
| server_name: str = "SERVER_NAME_NOT_PROVIDED", | |
| server_name: str = "third_party_code_running_from_unknown_server", |
We can align on whatever pattern we think is best.
|
This PR is paused while an alternative solution is explored in matrix-org/synapse-s3-storage-provider#134. |
|
Superseded by matrix-org/synapse-s3-storage-provider#134. |
This is a backwards-compatibility patch for modules that imported
LoggingContextfrom within Synapse. See matrix-org/synapse-s3-storage-provider#133.Spawned from a change in #18868, which has not yet gone out in a release. As such, re-uses the changelog from it.
Pull Request Checklist
EventStoretoEventWorkerStore.".code blocks.